-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disable yaml aliases for generateschema #7131
Conversation
rest_framework/renderers.py
Outdated
@@ -1053,6 +1053,8 @@ def __init__(self): | |||
assert yaml, 'Using OpenAPIRenderer, but `pyyaml` is not installed.' | |||
|
|||
def render(self, data, media_type=None, renderer_context=None): | |||
# disable yaml advanced feature 'alias' for clean, portable, and readable output | |||
yaml.Dumper.ignore_aliases = lambda *args: True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to do this without overriding ignore_aliases
on the Dumper
class? I'm wondering what the interaction is for anyone else who wishes to use yaml.dump
after this call.
(Just checking) but it is valid yaml being generated yes? So surely this last is a bug in whatever yaml parser these generators are using? |
The generator i use for testing is https://github.com/OpenAPITools/openapi-generator, which i believe is the defacto standard besides the swagger one. it will choke with errors like:
because it does not recognize the alias labels: content:
application/json:
schema: &id017
$ref: '#/components/schemas/SetPassword'
application/x-www-form-urlencoded:
schema: *id017
multipart/form-data:
schema: *id017 As the yaml is valid i think (no expert on that), you could say it is an upstream bug. So we can either do a upstream PR to fix this or only use the most common set of yaml features. In my opinion the aliases add complexity, non-descript names without much of a benefit except slightly lower schema file size. @kevin-brown Absolutely. Unfortunately there was no option for that. The last time i tried with an explicit Dumper it had no effect. Maybe i had an old pyyaml version. It works now. yaml/pyyaml#103 |
@tfranzel-cashlink Can you open an issue on openapi-generator to at least see what they say? |
Actually there is: OpenAPITools/openapi-generator#1593 apparently the problem lies deeper (openapig-generator -> Jackson lib -> SnakeYaml lib) Nonetheless, i have not seen any openapi spec yet that uses aliases. Upstream bug or not, it will not go through the de-facto standard generator, which makes the generated schema rather pointless to begin with. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I´m not sure I´m convinced by the de facto here. There are lots of tools out there. However...
Thanks for the extra references. I need to look into those and OpenAPI Generator but the immediate work-around is to use a custom renderer with your fix in place. That gets you going at least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tfranzel-cashlink It looks like you´ve resolved @kevin-brown´s worry so let´s have this. Thanks for the effort.
Would you be able to release a version with this change in? |
We're working on 3.12 currently. It'll be in that. Pip can help you if you're in a rush... |
@carltongibson as requested in #7096
when objects are reused in a structure, yaml creates aliases for those objects.
&id001
instead of proper names(will push fix once test fails)